-
Notifications
You must be signed in to change notification settings - Fork 465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WFCORE-6454] Use TCP/IP process communication for surefire plugin #5605
Conversation
testsuite/pom.xml
Outdated
@@ -154,6 +154,10 @@ | |||
See https://issues.apache.org/jira/browse/MNG-2496 --> | |||
<version.org.apache.xalan>2.7.3</version.org.apache.xalan> | |||
|
|||
<!-- This overrides default version specified in parent pom - reason is to enable TCP/IP communication, | |||
see https://maven.apache.org/surefire/maven-surefire-plugin/examples/process-communication.html --> | |||
<version.surefire.plugin>3.1.2</version.surefire.plugin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be in the root pom, as surefire is used outside the testsuite tree, and we'll likely lose track of this separate setting here.
The xalan version above should perhaps move to the root pom too (I'm not asking you to do that; I'm just explaining what may look like an inconsistency.) But it's a somewhat different case, as there the dependency is only used in the testsuite module. I think part of the idea with xalan was to keep the property out of places where people could misuse it to configure production code versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bstansberry thank you for your review! I've moved the version definition to the root pom as you suggested. I also included forkNode
definition in the surefire plugin configuration there too.
Let me know if you see some more issues, thanks! 🙂
This change utilizes new TCP/IP communication for surefire execution [1]. By default, the communication writes to STDOUT which may lead to similar messages in the execution output if something else also writes there at wrong time: ``` [WARNING] Corrupted STDOUT by directly writing to native stream in forked JVM 1. See FAQ web page and the dump file ... .dumpstream. ``` This change should mittigate such warnings and also should reduce number of open files during execution which may lead to crash on some systems with `nofile` limit set quite low. Truth is that this issue hasn't been seen in this upstream WildFly Core project, still I believe this change should be beneficial avoiding eventual issue in the future. [1] https://maven.apache.org/surefire/maven-surefire-plugin/examples/process-communication.html
83176b0
to
771d02d
Compare
Just for the record - surefire plugin version is gonna be updated by jboss/jboss-parent-pom#163. So we can remove the version overriding introduced in this PR once we move to the relevant parent pom in the future. |
Thanks @jstourac |
https://issues.redhat.com/browse/WFCORE-6454
This change utilizes new TCP/IP communication for surefire execution [1].
By default, the communication writes to STDOUT which may lead to similar messages in the execution output if something else also writes there at wrong time:
This change should mittigate such warnings and also should reduce number of open files during execution which may lead to crash on some systems with
nofile
limit set quite low.Truth is that this issue hasn't been seen in this upstream WildFly Core project, still I believe this change should be beneficial avoiding eventual issue in the future.
[1] https://maven.apache.org/surefire/maven-surefire-plugin/examples/process-communication.html